-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SIP-63 - Scala 3 Macro Annotations #80
base: main
Are you sure you want to change the base?
Conversation
|
||
/** Annotation classes that extend this trait will be able to transform the annotated definition. */ | ||
trait MacroAnnotation extends StaticAnnotation: | ||
/** Transforms a tree `definition` and optionally adds new definitions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cask and MainArgs are perhaps one of the more common libraries using "fake" macro annotations today, and I suspect they wouldn't be implementable using this current proposal as is
That because they require that multiple @annotated
methods be considered to generate a single combined routing object. In MainArgs you can have multiple @main
methods, and in Cask you can have multiple @cask.get
/@cask.post
/etc. routes.
In both cases, we currently have a expression-macro that looks at the class body, looks for all the @annotated def
s within it, and generates a single expression that references all of them.
These are different usage patterns than the use cases included in this proposal, but I don't think they are unreasonable. It's fundamentally the Registry Pattern, which e.g. is a very common use case for Python's @decorator
s which Scala macro annotations are very similar to. Consider this example from the popular Flask project:
from flask import Flask
app = Flask(__name__)
@app.route('/') # Registers `def index` in `app`
def index():
return 'Index Page'
@app.route('/hello') # Registers `def hello` in `app`
def hello():
return 'Hello, World'
Or Python's Click library:
import click
@click.group() # registers `def cli` with `click`
def cli():
pass
@cli.command() # registers `def initdb` with `cli`
def initdb():
click.echo('Initialized the database')
@cli.command()# registers `def dropdb` with `cli`
def dropdb():
click.echo('Dropped the database')
Is there some way we can support that sort of use case with Macro Annotations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe an aggregate version that only returns new trees to go at the end of the surrounding scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could have a macro annotation @flask
that looks for normal annotations @app.route(...)
in its scope
@flask def app: Flask = Flask()
@app.route("/")
def index() = "Index Page"
@app.route("/hello")
def hello() = "Hello, World"
it could then generate a main
as follows
class app {
<static> def main(args: Array[String]): Unit = {
app
.registerRoute("/", index)
.registerRoute("/hello", hello)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might also have it as
@flask val app: Flask = Flask()
and modify it to
val app: Flask =
Flask()
.registerRoute("/", index)
.registerRoute("/hello", hello)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the limitation we have is that we need to instantiate the annotation at compile time. If we have @app.route('/')
where app
is a reference to a local value that will only exist later (at runtime) we cannot instantiate it as a macro annotation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another approach that would follow closer the module initialization semantics of the Python version might be
object app extends Flask {
@route("/")
def index() = "Index Page"
@route("/hello")
def hello() = "Hello, World"
}
where the @root
macro annotation assumes it is inside a Flask
instance and generates something like
object app extends Flask {
val _ = (this: Flask).registerRoot("/" , index)
def index() = "Index Page"
val _ = (this: Flask).registerRoot("/hello", hello)
def hello() = "Hello, World"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolasstucki perhaps using the raw Python examples was a bit overly specific, so let's not over-fit on that exact syntax. Let's instead consider the Cask/MainArgs equivalents, which are a bit more idiomatic Scala:
// Cask
object MinimalApplication extends cask.MainRoutes{
@cask.get("/")
def hello() = {
"Hello World!"
}
@cask.post("/do-thing")
def doThing(request: cask.Request) = {
request.text().reverse
}
initialize()
}
// MainArgs
object Main{
@main
def foo(@arg(short = 'f', doc = "String to print repeatedly")
foo: String,
@arg(doc = "How many times to print string")
myNum: Int = 2,
@arg(doc = "Example flag")
bool: Flag) = {
println(foo * myNum + " " + bool.value)
}
@main
def bar(i: Int,
@arg(doc = "Pass in a custom `s` to override it")
s: String = "lols") = {
println(s * i)
}
def main(args: Array[String]): Unit = ParserForMethods(this).runOrExit(args)
}
In both Cask and MainArgs, we do a "batch" registration:
-
Cask's
initialize()
function takes an implicit macro that resolves the macro-annotations, generates the trees, then assigns it to a (private) mutable variable for use later -
MainArgs'
ParserForMethods
macro is an explicit macro call that resolves the macro annotations, generates the trees, and we ourselves use it as part ofdef main
(Mill actually has a similar design and implementation for it's T.command
definitions, that's very similar to how MainArgs works. It does a similar explicit macro call, albeit the macro call is performed in generated code so the user doesn't see it)
In fact, the @flask val app: Flask = Flask()
example you gave is a bit unnecessary: we can do the same thing without a macro annotation, via val app: Flask = Flask()
, by making Flask.apply
a macro (what MainArgs does) or making it take an implicit macro (what Cask does)
This is in contrast to the Python examples, which use mutable registries that each decorator appends to, similar to your (this: Flask).registerRoot("/" , index)
example. While that is a valid design, I think the use of mutable state is a bit un-idiomatic Scala, especially given the "immutable" approach of registering all the generated trees at once.
I think my ideal scenario would be
object Foo {
@bar
def qux() = "Index Page"
@baz
def cow() = "Hello, World"
}
Expanding to some kind of generalized (in pseudocode)
object Foo {
@bar
def qux() = "Index Page"
@baz
def cow() = "Hello, World"
${generateDefinition(expandMacro(@bar def qux), expandMacro(@baz def cow))}
}
That gives us flexibility: generateDefinition
can generate a:
object Main{ def main(args: Array[String]): Unit = ??? }
, for Scala 3@main
methodsdef main(args: Array[String]): Unit = ???
for MainArgs' `@main methodsval routes = Seq(???)
for Cask's routes
Perhaps the next questions would be, assuming bar
and qux
contain their own logic for macroExpand
, then:
- Who defines the
generateDefinition
logic? Is it on the enclosing template? Or some property ofbar
orqux
? - If we need to generate multiple definitions, e.g. we have an
object foo
that contains both@cask.get
s and MainArgs@main
methods, how do we ensure the@cask.get
s all get passed to Cask'sgenerateDefinition
logic, while all the MainArgs@main
s get passed to MainArgs'generateDefinition
logic?
Going with the mutable-registration-pattern approach sidesteps these questions - we can generate a separate statement before each annotated def that performs the registration when executed - it's just a bit awkward and unidiomatic. But if we cannot find a better solution then it's certainly an option.
This is all a bit convoluted, but I think it's doable to find an elegant approach here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that macro annotations might not be the best tool to get an elegant solution to this particular problem. Based on these examples, inline traits (report, prototype) are a much better fit.
Cask with inline traits
object MinimalApplication extends MainAutoRoot {
@get("/")
def hello() = "Hello World!"
@post("/do-thing")
def doThing(request: Request) = request.text().reverse
// generated code
// def routes: Route = Seq(
// Route("hello", hello),
// Route("doThing", doThing, ...),
// )
}
inline trait MainAutoRoot {
def routes: Route = generateRoutes // macro `generateRoutes` finds the routes defined in the class that extends `Routes`
}
trait MainRoot {
def roots: Roots
def main(...) = // do something with routes
}
- Who defines the generateDefinition logic? Is it on the enclosing template? Or some property of bar or qux?
In an inline trait, it would be the trait itself.
With inline traits it becomes simpler because there is a single point of expansion for the definition. The definition expansion can collect all (non-macro) annotated definitions in scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolasstucki I think using an inline trait sounds reasonable. But if that's our stance, then that basically cuts out Example 3: Main annotation
from the use cases of macro annotations: it doesn't make sense to say macro annotations provide a way to provide a worse version of MainArgs. We'll need to find other use cases to replace it, to make sure that macro annotations have enough motivating use cases to be worth it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it does not work for the particular API design choices of MainArgs
, it allows for many other kinds of main annotation use cases. For example, it can be used to implement the generalized main annotation in scala/scala3#19937.
#### Separate/incremental compilation | ||
As we transform the definitions after pickling, separate/incremental compilation is not affected by the macro annotations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if a macro annotated member of a trait
generates a new private val/var/lazy val
in the trait? That would still affect classes extending that trait, in a way that incremental compilation should be aware of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how we would support these. They might need to be disallowed.
There might be reasons to evaluate the inner macro annotation first, then the outer one. | ||
In that case, we have two options: we reverse the order of evaluation, or we add the ability to specify the order of evaluation of each macro annotation. | ||
|
||
### Allow new definitions to be macro annotated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what this means.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This refers to new definitions created by the macro expansion. I will reword it.
Please open a thread on contributors.scala-lang.org to gain community feedback, and link it to this proposal. |
Here is the contributors thread: https://contributors.scala-lang.org/t/scala-3-macro-annotations-sip-63-discussions/6593 |
…nnotation`) `MainAnnotation` and its implementation `newMain` predate `MacroAnnotation`. The `MacroAnnotation` is subsumed feature and allows much more flexibility. `MainAnnotation` and `newMain` could be reimplemented as a macro annotation in an external library. See SIP-63: scala/improvement-proposals#80
…nnotation`) `MainAnnotation` and its implementation `newMain` predate `MacroAnnotation`. The `MacroAnnotation` is subsumed feature and allows much more flexibility. `MainAnnotation` and `newMain` could be reimplemented as a macro annotation in an external library. See SIP-63: scala/improvement-proposals#80
One question that came up in the last SIP meeting is whether or not it's possible to implement #78 using macro annotations, and what it would look like if we did. @nicolasstucki do you have any insights there? We already have full implementations and test cases in the https://github.com/com-lihaoyi/unroll/ repo, so it should be possible to try it out using macro annotations and see if it can be made to work |
I wrote and example implementation of a strip down version of #78 for unrolling the last parameters of a However, we still have some limitations in the current implementation, such as missing the parameter macro annotations (we are woking on a prototype and should have something soonish). We might also me missing some helper functions in the reflection API to help us deal with this transformation more elegantly (for example a simple way to get the I have not tried with constructors yet. We will have a similar situation for class constructors. But, case class constructors parameter unrolling might not fit the current specification of macro annotations. As the annotation is on the parameter, the transformed tree will be the constructor of the case class, this only allows us to add methods in the case class itself but not in the compassion object. |
…nnotation`) `MainAnnotation` and its implementation `newMain` predate `MacroAnnotation`. The `MacroAnnotation` is subsumed feature and allows much more flexibility. `MainAnnotation` and `newMain` could be reimplemented as a macro annotation in an external library. See SIP-63: scala/improvement-proposals#80
…nnotation`) (#19937) `MainAnnotation` and its implementation `newMain` predate `MacroAnnotation`. The `MacroAnnotation` is subsumed feature and allows much more flexibility. `MainAnnotation` and `newMain` could be reimplemented as a macro annotation in an external library. See SIP-63: scala/improvement-proposals#80 ### When should this be removed? As an experimental feature, we can remove it in any patch release. We have 2 options: 1. Conservative: we wait until the next minor release to minimize the impact on anyone that was experimenting with this feature. (decided to take this approach in the Scala core meeting on the 13.03.2023) 2. ~Complete: We remove it as soon as we can, next patch release of 3.4 or 3.5. We also remove it from the next 3.3 LTS patch release.~
@jchyb and @hamzaremmal will take over the development of this SIP. |
This proposal was discussed at the last SIP meeting, but there was concern that the current proposal is currently too limited and did not have enough usecases as-is:
In particular, the fact that existing libraries like cask and mainargs cannot be ported to work as-is without requiring extra annotations seems problematic. If we look at the cask example from @lihaoyi again: // Cask
object MinimalApplication extends cask.MainRoutes{
@cask.get("/")
def hello() = {
"Hello World!"
}
@cask.post("/do-thing")
def doThing(request: cask.Request) = {
request.text().reverse
}
initialize()
} The current proposal would require an extra annotation on the To avoid this extra level of boilerplate, I'd like to suggest we introduce "inheritable macro annotation", concretely if we define MainRoutes in cask as: class caskGen extends InheritableMacroAnnotation { ... }
@caskGen trait MainRoutes { ... } Then we should desugar: object MinimalApplication extends cask.MainRoutes into: @caskGen object MinimalApplication extends cask.MainRoutes I believe inheritable macro-annotations could be useful in many situations. For example, if I want to memoize instances of an open class hierarchy, I'd like to be able to define: class memo extends InheritableMacroAnnotation { ... }
@memo trait Base so that anyone inheriting from |
No description provided.